Skip to content

fix(configurator): respect global studio token visibility - #408

Closed
jackgranatowski wants to merge 1 commit into
mainfrom
codex/refactor-obsuge-show/hide-token
Closed

fix(configurator): respect global studio token visibility#408
jackgranatowski wants to merge 1 commit into
mainfrom
codex/refactor-obsuge-show/hide-token

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

Motivation

  • Provide a single global UI preference to show/hide raw --sf-* token names across Studio views instead of forcing them on in Studio-only controls.
  • Keep FriendlyControl able to reveal token names locally via its existing rawOpen button without overriding the global preference.

Description

  • Add a persisted ui.showTokens boolean to the shared store in configurator/src/lib/store.svelte.js and accept it from sanitiseUiState in configurator/src/lib/uiState.js.
  • Persist showTokens in the App snapshot in configurator/src/App.svelte so the preference is saved to localStorage.
  • Add a header toggle button in configurator/src/components/Header.svelte to flip ui.showTokens at runtime.
  • Wire the Studio controls to the global state by passing showToken={ui.showTokens} (implemented as a derived showToken) from configurator/src/components/editors/StudioControls.svelte and configurator/src/components/editors/ColorStudio.svelte instead of unconditionally passing showToken.
  • Leave FriendlyControl.svelte behavior intact so its local rawOpen toggle still reveals token names locally when requested.
  • Add component tests in configurator/tests-components/studios.test.js that assert the global show/hide behavior and that FriendlyControl still exposes local raw token reveal.

Testing

  • Ran component tests: npm run test:components -- tests-components/studios.test.js, resulting in all tests in that file passing (12/12).
  • Ran static checks: npm run check, which completed with no errors (Svelte diagnostics reported two non-blocking warnings unrelated to these changes).

Codex Task

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jackgranatowski, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 28 minutes and 54 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3285c483-3963-491c-a99f-84f1ce3feb14

📥 Commits

Reviewing files that changed from the base of the PR and between f391ec2 and 708ea89.

📒 Files selected for processing (7)
  • configurator/src/App.svelte
  • configurator/src/components/Header.svelte
  • configurator/src/components/editors/ColorStudio.svelte
  • configurator/src/components/editors/StudioControls.svelte
  • configurator/src/lib/store.svelte.js
  • configurator/src/lib/uiState.js
  • configurator/tests-components/studios.test.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/refactor-obsuge-show/hide-token

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Respect global Studio token visibility preference
🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

Description

• Persist a global ui.showTokens preference in localStorage-backed UI state.
• Add a header toggle to show/hide raw --sf-* token names across Studios.
• Update Studio controls to follow global visibility while keeping FriendlyControl local reveal.
Diagram

graph TD
  H["Header toggle"] --> U["ui store"] --> D["derived showToken"] --> S["Studios"] --> F["FriendlyControl"]
  A["App snapshot"] --> L[("localStorage")] --> X["sanitiseUiState"] --> U
  subgraph Legend
    direction LR
    _db[("Persisted state")] ~~~ _ui["UI state"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Prop-drill from App (no store imports in components)
  • ➕ Makes dependencies explicit and avoids importing global store into leaf components
  • ➕ Potentially simpler to test components in isolation
  • ➖ Requires threading props through multiple component layers
  • ➖ More boilerplate and refactors when new global UI prefs are added
2. Svelte context for UI preferences
  • ➕ Avoids direct store imports while still preventing prop-drilling
  • ➕ Keeps UI preference wiring localized to a provider boundary
  • ➖ Adds an additional indirection/concept for contributors
  • ➖ Still requires careful SSR/test setup for context providers

Recommendation: Current approach (persisted shared store + derived showToken) is appropriate given existing architecture already relies on a shared ui store. The local FriendlyControl override behavior is preserved cleanly, and the added component tests reduce regression risk. Consider context/prop-drilling only if there’s a broader initiative to reduce direct store imports in leaf components.

Files changed (7) +47 / -8

Enhancement (3) +5 / -2
App.sveltePersist 'showTokens' in UI snapshot +1/-1

Persist 'showTokens' in UI snapshot

• Extends the localStorage UI snapshot to include 'showTokens' so the preference survives reloads. Keeps persistence aligned with sanitisation and store initialization.

configurator/src/App.svelte

Header.svelteAdd header toggle for token name visibility +1/-0

Add header toggle for token name visibility

• Adds a toggle button that flips 'ui.showTokens' and reflects state via aria-pressed and dynamic labels. Provides a single global control for showing/hiding raw token names.

configurator/src/components/Header.svelte

store.svelte.jsAdd persisted 'ui.showTokens' field +3/-1

Add persisted 'ui.showTokens' field

• Introduces 'showTokens' on the shared 'ui' state with a default sourced from saved UI state. Updates documentation/types for the persisted UI state shape.

configurator/src/lib/store.svelte.js

Bug fix (3) +11 / -4
ColorStudio.svelteWire ColorStudio controls to global 'ui.showTokens' +3/-1

Wire ColorStudio controls to global 'ui.showTokens'

• Imports the shared 'ui' store and derives 'showToken' from 'ui.showTokens'. Passes '{showToken}' to FriendlyControl instead of forcing token visibility on.

configurator/src/components/editors/ColorStudio.svelte

StudioControls.svelteRespect global token visibility in StudioControls +3/-2

Respect global token visibility in StudioControls

• Derives 'showToken' from 'ui.showTokens' and passes it into FriendlyControl. Prevents Studio-only wiring from overriding the new global preference.

configurator/src/components/editors/StudioControls.svelte

uiState.jsSanitise and accept 'showTokens' from persisted UI state +5/-1

Sanitise and accept 'showTokens' from persisted UI state

• Extends 'sanitiseUiState' to whitelist 'showTokens' when it’s a boolean. Keeps UI state loading robust to unknown/invalid persisted values.

configurator/src/lib/uiState.js

Tests (1) +31 / -2
studios.test.jsAdd component tests for global token visibility + local reveal +31/-2

Add component tests for global token visibility + local reveal

• Ensures tests reset 'ui.showTokens' and adds assertions that Studio token names follow the global setting. Verifies FriendlyControl can still reveal raw token names locally via its own toggle.

configurator/tests-components/studios.test.js

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Remediation recommended

1. Toggle bypasses some controls 🐞 Bug ≡ Correctness
Description
The new global ui.showTokens toggle is presented as a global preference, but some FriendlyControl
call sites still hardcode or omit showToken, so token-name visibility will remain inconsistent
(e.g., Essentials always shows token names; SmartSettings never shows them even when enabled). This
makes the header toggle not reliably reflect the stated “show/hide token names” behavior.
Code

configurator/src/components/Header.svelte[56]

+      <button onclick={() => (ui.showTokens = !ui.showTokens)} aria-pressed={ui.showTokens} title={ui.showTokens ? 'Hide token names' : 'Show token names'} aria-label={ui.showTokens ? 'Hide token names' : 'Show token names'}>{ui.showTokens ? '--' : '{ }'}</button>
Relevance

⭐⭐⭐ High

Team accepts UI consistency fixes; similar cross-view correctness changes were accepted (e.g.,
DomainPanel/TokenRow consistency in PRs #369,#395).

PR-#369
PR-#395
PR-#402

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The header introduces a global toggle and the store documents the preference as applying to
friendly/studio controls, but FriendlyControl only displays token names when its showToken prop is
truthy (or rawOpen). DomainPanel still forces showToken on for Essentials, and SmartSettings never
forwards ui.showTokens, so those areas cannot follow the global preference.

configurator/src/components/Header.svelte[48-57]
configurator/src/lib/store.svelte.js[76-79]
configurator/src/components/FriendlyControl.svelte[7-33]
configurator/src/components/DomainPanel.svelte[227-245]
configurator/src/components/SmartSettings.svelte[146-162]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A global UI preference (`ui.showTokens`) is introduced and toggled from the header, but not all `FriendlyControl` usages respect it. Some places force `showToken` on, and others never forward the global setting, so the toggle doesn’t consistently show/hide raw token names.

## Issue Context
- `ui.showTokens` is described as applying to “friendly/studio controls”.
- `FriendlyControl` only shows the token name when `showToken || rawOpen`.
- Some callers still use `showToken` shorthand (always true) or omit the prop, which bypasses the global preference.

## Fix Focus Areas
- Ensure `FriendlyControl` call sites that should follow the global preference pass `showToken={ui.showTokens}` (or a local `$derived(ui.showTokens)`), and remove any unconditional `showToken` shorthand where it should not override the global preference.
- Consider adding a fallback inside `FriendlyControl` (e.g., defaulting to `ui.showTokens` when `showToken` prop isn’t explicitly provided) to avoid missing future call sites, while keeping `rawOpen` behavior intact.

### Files/lines to update
- configurator/src/components/DomainPanel.svelte[227-245]
- configurator/src/components/SmartSettings.svelte[146-162]
- configurator/src/components/FriendlyControl.svelte[7-33]
- configurator/src/lib/store.svelte.js[76-79]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. No test for showTokens 🐞 Bug ⚙ Maintainability
Description
sanitiseUiState now accepts and returns the persisted showTokens boolean, but the existing ui-state
unit tests were not extended to cover this new field. This reduces regression protection for the new
persistence behavior.
Code

configurator/src/lib/uiState.js[R52-54]

+  if (typeof parsed.showTokens === 'boolean') {
+    out.showTokens = parsed.showTokens;
+  }
Relevance

⭐⭐⭐ High

They typically update ui-state tests when persistence schema changes (ui-state.test.js updates
merged in PR #367; testing emphasized in #313).

PR-#367
PR-#313

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
uiState.js now explicitly accepts boolean showTokens, but the ui-state test suite only covers
domain/outputMode/mode and malformed payloads; it contains no assertions for showTokens.

configurator/src/lib/uiState.js[14-56]
configurator/tests/ui-state.test.js[9-45]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`sanitiseUiState()` now supports a new persisted field (`showTokens`), but `configurator/tests/ui-state.test.js` doesn’t assert the new behavior.

## Issue Context
The validator is the single entry point for persisted UI state; adding a field without tests makes future refactors more likely to silently break persistence.

## Fix Focus Areas
- Add a test that a valid boolean `showTokens` round-trips.
- Add a test that non-boolean values (e.g. string/number/null) are ignored.

### Files/lines to update
- configurator/tests/ui-state.test.js[9-45]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant